Skip to content

update Om3#13

Closed
rhaegar325 wants to merge 36 commits into
v2from
update_OM3
Closed

update Om3#13
rhaegar325 wants to merge 36 commits into
v2from
update_OM3

Conversation

@rhaegar325

Copy link
Copy Markdown
Collaborator

No description provided.

@rhaegar325
rhaegar325 requested a review from rbeucher June 19, 2025 01:39
@rhaegar325
rhaegar325 marked this pull request as ready for review June 19, 2025 01:54

@rbeucher rbeucher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @rhaegar325,

Thank you. Can you please add some comments to your code. It would make things easier for us to review and keep track of what is happening.
I also would like to have some tests added.
I need to test the code so I can see that it is working properly.

Comment thread src/access_mopper/configurations.py Outdated
Comment thread src/access_mopper/configurations.py Outdated
Comment thread src/access_mopper/configurations.py Outdated
Comment thread src/access_mopper/configurations.py
Comment thread src/access_mopper/configurations.py Outdated
data = var.values
# CMOR 3.10 no longer accept NaN values as missing data. Use 1e20 instead.
# Fill Nan in data
var_fixed = var.fillna(1e20)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting from CMOR 3.10, NaN values are no longer accepted. Therefore, we replace NaN values before calling cmor.variable()


# Define CMOR variable
cmorVar = cmor.variable(cmor_name, variable_units, cmor_axes, positive=positive)
missing = 1e20

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

identify Missing value for CMOR

data = var.values
# CMOR 3.10 no longer accept NaN values as missing data. Use 1e20 instead.
# Fill Nan in data
var_fixed = var.fillna(1e20)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace NaN values before calling cmor.variable()


# Define CMOR variable
cmorVar = cmor.variable(cmor_name, variable_units, cmor_axes, positive=positive)
missing = 1e20

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

identify Missing value for CMOR


# Define CMOR variable
cmorVar = cmor.variable(cmor_name, variable_units, cmor_axes, positive=positive)
missing = 1e20

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

identify Missing value for CMOR

self.supergrid_file = supergrid_file
self.supergrid = xr.open_dataset(supergrid_file)
else:
raise ValueError("Couldn't find supergrid file")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update to adapt to the split test dataset (x.nc, y.nc).

Comment thread tests/test_mop.py
DATA_DIR / f"om3/2d/access-om3.mom6.2d.{cmor_name}.1mon.mean.1902_01.nc"
)
try:
model_om3.supergrid = glob.glob(str(DATA_DIR / "om3/supergrid/*.nc"))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because GitHub does not allow files larger than 100 MB, I split the original supergrid file into two smaller files (x.nc and y.nc), each containing only the required variables.
During testing, we now need to load and merge these two files before use.

Comment thread tests/test_mop.py
DATA_DIR / f"om3/3d/access-om3.mom6.3d.{cmor_name}.1mon.mean.1924_01.nc"
)
try:
model_om3.supergrid = glob.glob(str(DATA_DIR / "om3/supergrid/*.nc"))

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

@rbeucher

Copy link
Copy Markdown
Member

Hi @rhaegar325,

I have made some modifications to the code so most of this PR is not relevant anymore. However, I have cherry-picked some of your changes. I still need to implement the OM3 work but that should not be too much of an issue. Anyway, I am closing this now. Let's chat later.

@rbeucher rbeucher closed this Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants